Add vendor and image class UUID support #2409
Conversation
tomchy
requested review from
nordicjm,
de-nordic,
davidvincze and
d3zd3z
as code owners
Add a possibility to express vendor ID and image class ID inside image's TLVs. Signed-off-by: Tomasz Chyrowicz <[email protected]>
Allow to specify VID and CID for an image. Signed-off-by: Tomasz Chyrowicz <[email protected]>
Add a possibility to configure vendor ID and image class ID through Kconfig. Signed-off-by: Tomasz Chyrowicz <[email protected]>
tomchy
force-pushed
the
feature/bootutil/NCSDK-34175_Add_vid_cid_support_upstream
branch
from
ab8fffa to
fc4c042
Compare
|
|
||
| FIH_CALL(boot_uuid_vid_match, fih_rc, image_index, &img_uuid_vid); | ||
| if (FIH_NOT_EQ(fih_rc, FIH_SUCCESS)) { | ||
| FIH_SET(fih_rc, FIH_FAILURE); |
There was a problem hiding this comment.
This line should set uuid_vid_valid, not the assignment below.
|
|
||
| FIH_CALL(boot_uuid_cid_match, fih_rc, image_index, &img_uuid_cid); | ||
| if (FIH_NOT_EQ(fih_rc, FIH_SUCCESS)) { | ||
| FIH_SET(fih_rc, FIH_FAILURE); |
There was a problem hiding this comment.
This line should set the uuid_cid_valid, not the assignment below.
| if (len != sizeof(img_uuid_vid)) { | ||
| /* Vendor UUID is not valid. */ | ||
| rc = -1; | ||
| goto out; | ||
| } | ||
|
|
||
| rc = LOAD_IMAGE_DATA(hdr, fap, off, img_uuid_vid.raw, len); | ||
| if (rc) { | ||
| goto out; | ||
| } |
There was a problem hiding this comment.
If it was decided to wrap uuid in fih, these should probably also report fih failure; i do not understand a difference between a failure in uuid check and inability to perform the check. And I know that we are inconsistent here, and maybe that is a good point to discuss it.
| @@ -142,6 +142,8 @@ extern "C" { | |||
| * ... | |||
| * 0xffa0 - 0xfffe | |||
| */ | |||
| #define IMAGE_TLV_UUID_VID 0x80 /* Vendor unique identifier */ | |||
There was a problem hiding this comment.
These go above comment explaining vendor specific ids. If these are vendor specific ids, then they have in-proper codes.
There was a problem hiding this comment.
And should't really be here.
|
|
||
| static fih_ret boot_uuid_compare(const struct image_uuid *uuid1, const struct image_uuid *uuid2) | ||
| { | ||
| return fih_ret_encode_zero_equality(memcmp(uuid1->raw, uuid2->raw, |
There was a problem hiding this comment.
Are we sure we want to get fih heavy on uuid comparisons? What is the attack vector here? As far as i cen tell they are already protected tlvs and, as such, are signed with image, and why would you bypass uid check? You would have to bypass signature first.
According to the PSA Platform Security Boot Guide:
The manifest (which can be interpreted as image header and additional metadata stored inside TLVs) must include:
Instance ID.
This PR introduces two identifiers to meet those requirements: